Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update app icons for NewDot branding #2635

Merged
merged 5 commits into from
Jul 14, 2021
Merged

Update app icons for NewDot branding #2635

merged 5 commits into from
Jul 14, 2021

Conversation

shawnborton
Copy link
Contributor

@shawnborton shawnborton commented Apr 29, 2021

cc @AndrewGable this should be a good head start on getting all of the app icons updated.

Details

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/161805

Tests

QA Steps

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

image

Desktop

image

image

iOS

image

image

Android

@shawnborton shawnborton requested a review from a team as a code owner April 29, 2021 19:58
@MelvinBot MelvinBot requested review from Luke9389 and removed request for a team April 29, 2021 19:58
Luke9389
Luke9389 previously approved these changes Apr 29, 2021
@Luke9389
Copy link
Contributor

Hey @shawnborton, I see this is a [WIP]. Are you wanting to add more icons?

@shawnborton
Copy link
Contributor Author

Hey @Luke9389 - thanks for the speedy review! I'm mostly just curious for Andrew's input on what to do for app store descriptions, etc. Stay tuned - we'll let you know if there are more changes coming.

@AndrewGable
Copy link
Contributor

The app store pulls the app icon for some of the listing and then the other part of the listing I have to go and upload manually myself (similar to the app store screenshots)

@shawnborton shawnborton self-assigned this Apr 29, 2021
@shawnborton
Copy link
Contributor Author

Okay cool, I think that means we're all good here then.

@shawnborton
Copy link
Contributor Author

shawnborton commented Apr 29, 2021

Hmm I am having a hard time getting the iOS app up and running for testing. Seeing this:

The following build commands failed:
        CompileC /Users/shawnborton/Library/Developer/Xcode/DerivedData/ExpensifyCash-hfttzailbkhegigkkorhlarxjpit/Build/Intermediates.noindex/Pods.build/Debug-iphonesimulator/Flipper-Folly.build/Objects-normal/x86_64/DistributedMutex.o /Users/shawnborton/Expensify/Expensify.cash/ios/Pods/Flipper-Folly/folly/synchronization/DistributedMutex.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Is someone able to help me test?

Maybe this is helpful: facebook/flipper#2215

@Luke9389
Copy link
Contributor

Have you pulled from master and then reinstalled Pods (cd iOS && rm -rf ./Pods && pod install && cd ..)? That's usually my go-to

@shawnborton
Copy link
Contributor Author

Still getting the same error :( going to post in Slack and see if anyone has any ideas.

@Luke9389
Copy link
Contributor

Oh also doing the same thing with node_modules can be helpful. Total shot in the dark though

@shawnborton shawnborton changed the title [WIP] Update app icons for NewDot branding Update app icons for NewDot branding Apr 30, 2021
@shawnborton
Copy link
Contributor Author

@Luke9389 @AndrewGable taking this off hold, but the caveat being that I can't get the Android app to run locally so I have no way of testing this over there. Could either of you two help me out?

@Luke9389
Copy link
Contributor

Sure, I'll give it a spin 👍

@Luke9389
Copy link
Contributor

Luke9389 commented Apr 30, 2021

The icons are looking good but my emulator has been stuck on the loading screen (second pick) for a few mins now. Not seeing any errors in the Metro console though. @AndrewGable have you seen this before?

Edit: nvm, forgot ngrok 🤦 . Testing now.

Edit2: Getting tons of 502 errors...
Screen Shot 2021-04-30 at 3 39 47 PM
Screen Shot 2021-04-30 at 3 39 23 PM

@shawnborton shawnborton changed the title Update app icons for NewDot branding [HOLD] Update app icons for NewDot branding May 3, 2021
@shawnborton
Copy link
Contributor Author

Adding a quick hold to this until some discussion about the timing of this is sorted out.

@shawnborton
Copy link
Contributor Author

Closing this one as we are no longer going to use this new icon.

@shawnborton shawnborton closed this May 6, 2021
@MitchExpensify
Copy link
Contributor

Ok to reopen as part of N6?

@shawnborton shawnborton reopened this Jul 9, 2021
@shawnborton
Copy link
Contributor Author

I think it's okay to reopen, but perhaps @Luke9389 or someone can help clean this up and take it over the finish line for me?

@Luke9389
Copy link
Contributor

Luke9389 commented Jul 9, 2021

Hey @shawnborton, happy to pick this up.

So @AndrewGable you mentioned above that you'll need to manually upload this icon to the app store. When should that happen? Do you want to coordinate with me so that this PR gets deployed and the icons get updated at the same time? I can ping you when this is ready for merge.

My game plan for this PR is to clean up the merge conflicts and test on all the platforms.

@shawnborton
Copy link
Contributor Author

Good question, I'm not entirely sure the best way to time things but I know we will be changing the name of the app as well, which causes us to push to the app store for this newsletter launch. For more context: https://github.com/Expensify/Expensify/issues/170114

@Luke9389 Luke9389 changed the title [HOLD] Update app icons for NewDot branding Update app icons for NewDot branding Jul 12, 2021
@AndrewGable
Copy link
Contributor

@Luke9389 - Feel free to change/merge any icons, names, colors, etc all in this PR. We will update the assets (including new icons) in the issue @shawnborton linked 👉 https://github.com/Expensify/Expensify/issues/170114 when we publish the app to the "live" store which requires a mobile deployer to configure the release.

@Luke9389
Copy link
Contributor

OK, I've tested this on all platforms. Gonna assign pullerbear to get this merged.

@Luke9389 Luke9389 requested a review from a team July 13, 2021 20:16
@Luke9389 Luke9389 self-assigned this Jul 13, 2021
@Luke9389 Luke9389 self-requested a review July 13, 2021 20:16
@MelvinBot MelvinBot requested review from sketchydroide and removed request for a team July 13, 2021 20:16
@Luke9389 Luke9389 requested review from a team and removed request for Luke9389 July 13, 2021 20:16
@MelvinBot MelvinBot requested review from pecanoro and removed request for a team July 13, 2021 20:17
@Luke9389 Luke9389 removed the request for review from pecanoro July 13, 2021 20:21
@Luke9389
Copy link
Contributor

somehow double-dipped on pullerbear 😬

@Luke9389 Luke9389 merged commit 484e529 into main Jul 14, 2021
@Luke9389 Luke9389 deleted the shawn-updateAppIcon branch July 14, 2021 17:32
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.77-6🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

@Luke9389 Luke9389 restored the shawn-updateAppIcon branch July 14, 2021 20:46
@Luke9389 Luke9389 deleted the shawn-updateAppIcon branch July 14, 2021 20:46
@mvtglobally
Copy link

Title:
PR2635 - UI looks different for start page .cash

Expected Result:
The start/login page .cash should be same as shown in PR2635

Actual Result:
UI looks different for start/login page .cash

Action Performed:

  1. Launch App

Workaround:
Unknown

Platform:
Where is this issue occurring?

Web
iOS ✔️
Android
Desktop App
Mobile Web

Version Number: 1.0.78-0

@shawnborton
Copy link
Contributor Author

We recently merged a PR that changed the formatting of the log in page, so everything should be good here.

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.79-4🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants